Torch integration #692
Open
Torch integration #692
+6,102
−4,072
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d style consistency (#725) - [x] Fix license text: "MIT license" → "MIT License" in multiple files - [x] Rename files with "_2" suffix and update references - [x] Add missing license headers - [x] Fix header guards to follow MSCCLPP_EXT_<FILE_NAME>_HPP_ format - [x] Fix enum naming consistency - [x] CommResult enum to CamelCase - [x] CollectiveBufferMode enum to CamelCase (Any, InPlace, OutOfPlace) - [x] AlgorithmType enum to CamelCase (Native, DSL) - [x] Fix comment in src/core/gpu_utils.cc - [x] Fix LogSubsys::COUNT case in src/core/include/logger.hpp - [x] Add explanatory comment - [x] Add [[fallthrough]] attribute - [x] Apply clang-format - [x] Remove _codeql_detected_source_root file and add to .gitignore - [x] Update documentation paths - [x] Fix NCCL library paths: build/apps/nccl/ → build/lib/ - [x] Fix test binary paths: ./test/ → ./bin/ <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: chhwang <[email protected]>
Contributor
Author
|
/azp run mscclpp-ut |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run mscclpp-ut |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run mscclpp-ut |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Contributor
Author
|
/azp run mscclpp-ut |
|
Azure Pipelines successfully started running 1 pipeline(s). |
chhwang
approved these changes
Jan 19, 2026
Contributor
chhwang
left a comment
chhwang
left a comment
There was a problem hiding this comment.
Let's tackle comments in another PR
| __all__ += algorithm.__all__ | ||
| __all__ += comm.__all__ | ||
| __all__ += compiler.__all__ | ||
| __all__ += buffer.__all__ |
| from mscclpp._mscclpp import ( | ||
| Algorithm as _Algorithm, | ||
| DslAlgorithm as _DslAlgorithm, | ||
| AlgorithmType as _AlgorithmType, |
Contributor
There was a problem hiding this comment.
Python-binded objects would be named differently in the future (e.g., adding Cpp* prefix) to avoid confusion
| ) | ||
| ).hexdigest() | ||
|
|
||
| plan_dir = os.environ.get("MSCCLPP_EXECUTION_PLAN_DIR", Path.home() / ".cache/mscclpp") |
Contributor
There was a problem hiding this comment.
- All env variables should be collected and documented in
env.cpp, even if it's used only at Python level. - Is execution plan a part of "cache", meaning, this belongs to the system not to the user?
- I'd prefer
MSCCLPP_CACHE_DIRenv and we internally decide what to put where.
| recompilation. | ||
| The cache location can be configured via the `MSCCLPP_NATIVE_CACHE_DIR` | ||
| environment variable (defaults to `~/.cache/mscclpp/native`). |
Contributor
There was a problem hiding this comment.
I'd prefer MSCCLPP_CACHE_DIR env and we internally decide what to put where.
| rocm_home = os.environ.get("ROCM_HOME") | ||
| return os.path.join(rocm_home, "bin/hipcc") if rocm_home else "hipcc" | ||
| else: | ||
| cuda_home = os.environ.get("CUDA_HOME") |
Contributor
There was a problem hiding this comment.
Why not MSCCLPP_CUDA_HOME/MSCCLPP_ROCM_HOME?
|
|
||
| from .algorithm_collection_builder import * | ||
|
|
||
| __all__ = algorithm_collection_builder.__all__ |
| break; | ||
| case DataType::FP8_E4M3: | ||
| case DataType::FP8_E5M2: | ||
| // FP8 is not supported in CUDA execution kernel. |
Contributor
There was a problem hiding this comment.
Should we warn or throw here?
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Contributor
Author
|
Will resolve all the comments in another PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reorganize current native algorithm implementation and DSL algorithm implementation.
Provide unified API for DSL algo and native algo and provide interface to tune the algo
Provide interface for pytorch integration with native API and DSL